Skip to content

Conversation

Buratti
Copy link
Contributor

@Buratti Buratti commented May 28, 2024

Adds support for multiple actions and resources in the same Policy's Statement

Motivation:

It is common practice to return a statement that describes which integrations are available to the authorized principal. Such statement usually allows the "execute-api:Invoke" action on a list of integrations ARN.
The current design of the APIGatewayLambdaAuthorizerPolicyResponse didn't allow that and instead forces the user to generate a long list of statements, each allowing access to just one integration.

Modifications:

action and resource are now arrays, a new initializer has been added which takes arrays instead of strings. The existing initializer has been changed to automatically initialize a one-element array.
A new test has been added to validate the correct encoding and decoding.

Copy link
Contributor

@sebsto sebsto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for having added this. I just suggested one small change


public init(action: String, effect: Effect, resource: String) {
self.action = [action]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would call the other init() method to avoid duplicating code

@sebsto sebsto self-assigned this May 28, 2024
@sebsto sebsto added the kind/enhancement Improvements to existing feature. label May 28, 2024
@Buratti
Copy link
Contributor Author

Buratti commented May 28, 2024

Hi @sebsto I have ran the soundness.sh script but apparently it changed a few files that weren't supposed to be edited in this PR. You want me to revert the last commit?

Copy link
Contributor

@sebsto sebsto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the proposed changes.
I would keep only the change in the file Sources/AWSLambdaEvents/APIGatewayLambdaAuthorizers.swift

Can you revert the other changes made by Swift Format ? I have another PR in progress that takes care of these.

Thanks

@sebsto
Copy link
Contributor

sebsto commented May 29, 2024

@swift-server-bot test this please

1 similar comment
@ktoso
Copy link
Contributor

ktoso commented May 29, 2024

@swift-server-bot test this please

@ktoso
Copy link
Contributor

ktoso commented May 29, 2024

@swift-server-bot add to allowlist

@sebsto
Copy link
Contributor

sebsto commented May 29, 2024

@yim-lee can you help and give me permanent authorisation to trigger a test on any PR on this project and on Lambda runtime project ? Thank you

@sebsto
Copy link
Contributor

sebsto commented May 29, 2024

@Buratti Can you also pull and merge the changes from the main branch ? I just merged a PR that has non-conflicting changes on the LambdaAuthorizer.swift file. Thank you and sorry for the trouble

@Buratti
Copy link
Contributor Author

Buratti commented May 29, 2024

Hi @sebsto! Sure! I'll push the changes in a few hours.

@yim-lee
Copy link
Contributor

yim-lee commented May 29, 2024

can you help and give me permanent authorisation to trigger a test on any PR on this project and on Lambda runtime project

@sebsto Done. You should have the same privileges as @ktoso.

Copy link
Contributor

@sebsto sebsto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@sebsto
Copy link
Contributor

sebsto commented May 29, 2024

@Buratti one more little soundness fix

@Buratti
Copy link
Contributor Author

Buratti commented May 29, 2024

Hi @sebsto! I'm sorry, forgot to run it again. It should be good now!

@sebsto sebsto merged commit 212c6dc into swift-server:main May 29, 2024
@sebsto
Copy link
Contributor

sebsto commented May 29, 2024

Merged. Thank you !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement Improvements to existing feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants